Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix custom resources often missing from Quick Load dialog #93909

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

vivoxel
Copy link
Contributor

@vivoxel vivoxel commented Jul 3, 2024

Fixes #93813

Fixed the assignment to fc.resource_script_class and fc.type

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, haven't tested yet though but makes sense

@akien-mga
Copy link
Member

Rebased to edit the commit message. Please make sure to use a meaningful commit message for future PRs, see https://github.com/godotengine/godot/blob/master/CONTRIBUTING.md#format-your-commit-messages-with-readability-in-mind

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to fix the bug from a quick test, though the bug's apparent randomness is hard to correlate with how it's being fixed.

The code could maybe be made slightly cleaner but the only ideas I have may actually be more confusing that the current one, e.g.:

FileCache fc;
fc.type = split[1].get_slice("/", 0);
fc.resource_script_class = split[1].get_slice("/", 1); // Empty string if there's no separator.

I think I prefer the has check which makes it clear it's not always present.

@akien-mga akien-mga merged commit 5f0a2dd into godotengine:master Jul 4, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom resources often missing from Quick Load dialog.
3 participants